Packaging: Use of pyproject.toml and move to minimal setup.py for modernization by fmalatino · Pull Request #159 · NOAA-GFDL/NDSL

fmalatino · 2025-05-29T17:25:38Z

Description
This PR moves the metadata and configuration for the package to a pyproject.toml file, as is prescribed by Deprecate legacy setup.py develop mechanism for pip install --editable #11457. The previous setup.py file is now minimal. `

How Has This Been Tested?
Tested using the current CI workflow

Checklist:

My code follows the style guidelines of this project
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
My changes generate no new warnings
Any dependent changes have been merged and published in downstream modules
New check tests, if applicable, are included

romanc

Two questions:

Is there a way to move the non-optional dependencies also to the pyproject.toml and get rid of setup.py?
Did you test local developer workflows, e.g. installing NDSL (and gt4py/dace) editable?

fmalatino · 2025-05-30T13:41:50Z

Two questions:

Is there a way to move the non-optional dependencies also to the pyproject.toml and get rid of setup.py?

Did you test local developer workflows, e.g. installing NDSL (and gt4py/dace) editable?

@romanc

Unless we want to pull dace and gt4py from their respective repositories, we will need to continue to use the dynamic dependencies definitions
Yes I tested this by comparison to what is done currently when running pip install -e .[extras] on develop and confirming that both generate editable installations. In both cases, to achieve and editable install of either gt4py and dace one would have to go to their respective directories and run a pip install -e . with either install method.

romanc · 2025-05-30T15:31:44Z

Thanks Frank, that sounds good.

Another question: what do you think about moving the configuration from setup.cfg to pyproject.toml too?

oelbert

👍

fmalatino · 2025-07-09T15:14:00Z

Thanks Frank, that sounds good.

Another question: what do you think about moving the configuration from setup.cfg to pyproject.toml too?

@romanc, given the current issues between Flake8 and parsing its configuration from a pyproject.toml file, I vote to leave it as is, also given that we are holding on to the setup.py file as well. When we are able to get rid of the setup.py file, I think it will make sense to also get rid of the setup.cfg file. Thoughts?

romanc · 2025-07-09T20:40:56Z

Thanks Frank, that sounds good.
Another question: what do you think about moving the configuration from setup.cfg to pyproject.toml too?

@romanc, given the current issues between Flake8 and parsing its configuration from a pyproject.toml file, I vote to leave it as is, also given that we are holding on to the setup.py file as well. When we are able to get rid of the setup.py file, I think it will make sense to also get rid of the setup.cfg file. Thoughts?

I'm unaware about problems between flake8 and reading the configfile. That sounds weird to me given that pyproject.toml is the new standard. But yeah, if there are issues, we can iterate and do follow-up PRs.

Is this ready to be merged/reviewed? Asking because the PR is still marked as draft...

romanc · 2025-07-10T08:56:10Z

Two more comments:

Re flake8: maybe double check. The version was updated (3.x -> 7.x) two weeks ago in PR Update pre-commit hooks #172. Maybe that helped?
I think it would make sense to bump setuptools to >=77.0.3 and then update the license / license-files information as suggested in the official guide. This would allow to cleanly include the license information of ndsl/viz/fv3.

fmalatino · 2025-07-10T14:19:34Z

Two more comments:

Re flake8: maybe double check. The version was updated (3.x -> 7.x) two weeks ago in PR Update pre-commit hooks #172. Maybe that helped?

I think it would make sense to bump setuptools to >=77.0.3 and then update the license / license-files information as suggested in the official guide. This would allow to cleanly include the license information of ndsl/viz/fv3.

I will continue to dig on this, my focus is currently on finishing out the analogous changes in pace. Once I am ready for review I will change the status of this PR, I apologize for the confusion.

romanc

I've been reading up on python packing and I agree, this is the most standard way add local package dependencies. It will support all current workflows and prep us for the future to come.

This is starting to look nice. To be clear, can we get rid of setup.cfg now? Otherwise just some minor suggestions inline.

(And sorry for the un-asked review. I was just monitoring activity and thought I'd share my thoughts / endorsement for this move to pyproject.toml.

romanc

Coming along nicely! 🚀

….yaml

romanc

Nice! Thanks for bringing us up to speed with recent python config developments 🚀

…ernization (NOAA-GFDL#159) * Moving to pyproject.toml and minimal setup.py * Linting * Fixing setup.py syntax * Moving setup.cfg contents to pyproject.toml * Adding license information for ndsl/viz/fv3/README.md * Adding private classifier to pyproject.toml * Removing setup.cfg * Fixing unit test workflow * Changing install option from 'develop' to 'dev' * Adding linting for yamls * Adding Flake8-pyproject as additional dependency in pre-commit-config.yaml

Moving to pyproject.toml and minimal setup.py

8dd3ec5

fmalatino requested review from FlorianDeconinck, bensonr, oelbert and romanc May 29, 2025 17:25

Linting

8d90eda

romanc reviewed May 30, 2025

View reviewed changes

oelbert previously approved these changes May 30, 2025

View reviewed changes

fmalatino marked this pull request as draft June 13, 2025 14:25

fmalatino added 2 commits July 9, 2025 11:08

Merged in develop branch

6723e61

Fixing setup.py syntax

fd50a49

fmalatino dismissed oelbert’s stale review via fd50a49 July 9, 2025 15:09

fmalatino added 3 commits July 14, 2025 14:33

Moving setup.cfg contents to pyproject.toml

1451347

Merge branch 'develop' into packaging

43b89fa

Adding license information for ndsl/viz/fv3/README.md

8682df3

romanc reviewed Jul 14, 2025

View reviewed changes

Comment thread setup.py Outdated

Comment thread setup.py Outdated

Comment thread pyproject.toml Outdated

Comment thread pyproject.toml Outdated

fmalatino added 4 commits July 15, 2025 10:07

Adding private classifier to pyproject.toml

42f0f94

Removing setup.cfg

2dba80b

Fixing unit test workflow

facfb12

Changing install option from 'develop' to 'dev'

ee755b2

fmalatino requested review from oelbert and romanc July 15, 2025 15:10

fmalatino marked this pull request as ready for review July 15, 2025 15:10

romanc reviewed Jul 15, 2025

View reviewed changes

Comment thread .pre-commit-config.yaml Outdated

Comment thread pyproject.toml Outdated

romanc reviewed Jul 15, 2025

View reviewed changes

Comment thread pyproject.toml Outdated

fmalatino added 2 commits July 15, 2025 11:56

Adding linting for yamls

0a74c08

Adding Flake8-pyproject as additional dependency in pre-commit-config…

b20dfbf

….yaml

fmalatino requested a review from romanc July 16, 2025 19:17

romanc approved these changes Jul 16, 2025

View reviewed changes

fmalatino merged commit 23147a1 into NOAA-GFDL:develop Jul 17, 2025
5 checks passed

This was referenced Jul 18, 2025

ci: Re-enable flake8's F401 rule: no unused imports #180

Merged

Move to using pyproject.toml for use of pip >= 25.2 for a modern build system #138

Closed

romanc mentioned this pull request Aug 21, 2025

NDSL Documentation via MkDocs #205

Merged

fmalatino deleted the packaging branch August 26, 2025 17:17

Conversation

fmalatino commented May 29, 2025

Uh oh!

romanc left a comment

Choose a reason for hiding this comment

Uh oh!

fmalatino commented May 30, 2025

Uh oh!

romanc commented May 30, 2025

Uh oh!

oelbert left a comment

Choose a reason for hiding this comment

Uh oh!

fmalatino commented Jul 9, 2025

Uh oh!

romanc commented Jul 9, 2025

Uh oh!

romanc commented Jul 10, 2025

Uh oh!

fmalatino commented Jul 10, 2025

Uh oh!

romanc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romanc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

romanc left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants